-
Notifications
You must be signed in to change notification settings - Fork 429
UI Tests for auth flows #3959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UI Tests for auth flows #3959
Conversation
NB: tests are actually inputing username and password in login screen (But won't work if 2FA is required)
…to auth_ui_tests # Conflicts: # native/SampleApps/AuthFlowTester/AuthFlowTester.xcodeproj/project.pbxproj
The tests expects a test_config.json with login host, consumer keys, user credentials Tests configure the login host (if needed) then do a login / request / revoke / request / logout We need the static boot config to be configured from the test also but we cannot bring the dev menu currently (no shake action possible from a UI test)
…into auth_ui_tests
- default scopes, subset or all scopes - ECA issuing JWT or not - Web server flow or user agent flow
Generated by 🚫 Danger |
|
An actual bug fix revealed by some of the tests: 8d3ab2a |
Clang Static Analysis Issues
Generated by 🚫 Danger |
Codecov Report❌ Patch coverage is ❌ Your patch status has failed because the patch coverage (54.38%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## dev #3959 +/- ##
==========================================
+ Coverage 61.68% 63.30% +1.62%
==========================================
Files 255 252 -3
Lines 22733 22137 -596
==========================================
- Hits 14023 14014 -9
+ Misses 8710 8123 -587
🚀 New features to boost your workflow:
|
|
I'm thinking of having a button that dumps all the data from the AuthFlowTester screen in an alert. It would not be very useful when manually testing but would speed up the UI tests. Collecting all the static texts on the screen takes a while. |
…edentialsView / OAuthConfigurationView and JwtTokenView Leveraging the new buttons in UI tests (to run faster)
Done here: 3a3c10b |
| // Restart authentication with the updated configuration | ||
| if ([presentedViewController isKindOfClass:[SFLoginViewController class]]) { | ||
| [[SFUserAccountManager sharedInstance] restartAuthenticationForViewController:(SFLoginViewController *)presentedViewController]; | ||
| [[SFUserAccountManager sharedInstance] restartAuthenticationForViewController:(SFLoginViewController *)presentedViewController recreateAuthRequest:YES]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Login options might change the consumer key / callback url / scopes so we need to restart the oauth request otherwise the new settings will not be used.
| Spacer() | ||
| Image(systemName: isExpanded ? "chevron.up" : "chevron.down") | ||
| .foregroundColor(.secondary) | ||
| Button(action: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding an import button:
- manual tester could paste a boot config
- UI tests make use of it - it saves on a few taps and speed up the tests a bit
| [self dismissViewControllerAnimated:YES completion:^{ | ||
| if ([self.delegate respondsToSelector:@selector(loginViewControllerDidReload:)]) { | ||
| [self.delegate loginViewControllerDidReload:self]; | ||
| if ([self.delegate respondsToSelector:@selector(loginViewControllerDidChangeLoginOptions:)]) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reload is not enough - we need the auth request within the auth session to be recreated (see earlier comment).
| AUTH040 /* RestApiTestView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AUTH041 /* RestApiTestView.swift */; }; | ||
| AUTH042 /* OAuthConfigurationView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AUTH043 /* OAuthConfigurationView.swift */; }; | ||
| AUTH049 /* JwtAccessView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AUTH050 /* JwtAccessView.swift */; }; | ||
| AUTH049 /* JwtTokenView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AUTH050 /* JwtTokenView.swift */; }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Better name for the view class.
| import SalesforceSDKCore | ||
|
|
||
| // MARK: - Labels Constants | ||
| public struct JwtTokenLabels { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulled the labels into a struct.
- they are used also when exporting details as JSON.
- they are also used in UI tests.
| import SalesforceSDKCore | ||
|
|
||
| // MARK: - Labels Constants | ||
| public struct CredentialsLabels { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pulled the labels into a struct.
- they are used also when exporting details as JSON.
- they are also used in UI tests.
| var clientId: String | ||
| } | ||
|
|
||
| class AuthFlowTesterMainPageObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PageObject for the AuthFlowTester main screen (and also action sheets that you can bring up from the bottom tool bar).
| import XCTest | ||
| import SalesforceSDKCore | ||
|
|
||
| class LoginPageObject { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PageObject for the login screen as well as login server action sheet and login options action sheet.
| @@ -0,0 +1,87 @@ | |||
| { | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A file like this one is needed for the UI tests to run.
It contain a login host, app settings and user credentials.
| // MARK: - Configuration Utility | ||
|
|
||
| /// Utility class to parse and access test configuration from command-line arguments | ||
| class TestConfigUtils { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Class responsible for getting configuration from test_config.json.
Tests that are missing config should fail fast
The ones involving JWT-based access token were not working: - because there is server-side bug preventing the beacon app to be configured properly (switched to sdb38 org which runs more recent code where the issue has been resolved) - test code was wrongly expecting beacon consumer key instead of beacon child consumer key in the JWT
… opaque token even when sfap_api scope was granted (bug filed)
… use static configuration Always setting static configuration (so that tests can assert successfully)
…n the migration or multi users tests) => to speed things up a bit and avoid being redundant Added some login tests that makes use of a dynamic configuration Updated overview.md
|
Not ready to merge yet. However all the login tests are now passing locally. |
…sed when constructing the approval URL
Expecting more users in test_config.json Using different users in different test suites
Adding rollbacks in several migration tests
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||
brandonpage
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! There are an impressive amount of login scenarios covered here (and I have a lot of work to do to match).
I am a little surprised how how many SDK updates were necessary to apply the consumer key and redirect URI. Perhaps a MVVM overhaul like we did on Android could be helpful for organization in the future.
At some point I will steal your PageObject improvements and add them to the UITest repo so we can consumer them from there someday.
| dynamicScopes: String, | ||
| useWebServerFlow: Bool, | ||
| useHybridFlow: Bool, | ||
| supportWelcomeDiscovery: Bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the use of trailing commas, I didn't know that was in Swift .
Getting a 403 when invoking the single access bridge API with a beacon app access token (file a bug with auth team).
Adding UI tests for auth flows using the new AuthFlowTester app.
The tests expect a test_config.json file which should contain:
There are a number of login tests to the various Connected Apps, External Client Apps and Beacon Apps, with web server flow or user agent flow, with and without hybrid flow, with default scopes, a subset of scopes or all scopes.
There are also a number of migration tests across apps, and also on the same app with different scopes.
For information on the org test setup see the doc "13.2 Test Org Setup for Authentication Testing".
Notes
It's not ready to merge